Skip to content

Conversation

@DarshitChanpura
Copy link
Member

@DarshitChanpura DarshitChanpura commented Oct 13, 2025

Description

Implements resource-access-control for workflow and workflow_state.

Related Issues

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@DarshitChanpura
Copy link
Member Author

CI will resolve once: opensearch-project/security#5677 is merged.

Signed-off-by: Darshit Chanpura <[email protected]>
@DarshitChanpura
Copy link
Member Author

CI blocked by #1252

Copy link
Member

@owaiskazi19 owaiskazi19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1st iteration

build.gradle Outdated
testImplementation("org.junit.jupiter:junit-jupiter:${junitJupiterVersion}")
testImplementation("com.fasterxml.jackson.datatype:jackson-datatype-jsr310:${versions.jackson_databind}")

testImplementation 'org.awaitility:awaitility:4.3.0'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constant for the version

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack.

// | Output for ./bin/opensearch-plugin:ERROR: transport error 202: connect failed: Connection refused
// | ERROR: JDWP Transport dt_socket failed to initialize, TRANSPORT_INIT(510)
// | JDWP exit error AGENT_ERROR_TRANSPORT_INIT(197): No transports initialized [src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c:700].
// So instead, we listen to a debugger by saying server=y and suspend=n
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove the comments?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, let's keep it to understand what these changes mean.

* @return true if resource-authz should be used, false otherwise
*/
public static boolean shouldUseResourceAuthz(String resourceType) {
var client = ResourceSharingClientAccessor.getInstance().getResourceSharingClient();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make it explicit

Suggested change
var client = ResourceSharingClientAccessor.getInstance().getResourceSharingClient();
ResourceSharingClient client = ResourceSharingClientAccessor.getInstance().getResourceSharingClient();

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compilation may fail if security plugin is absent and by extension SPI is absent as well.
(because of the import statement)

Request request,
ActionListener<Response> listener
) {
if (subject == null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

subject would be null with the first constructor call, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't expect subject to be null, this is added to ensure robustness.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Onboard flow-framework plugin to Centralized Resource AuthZ framework

2 participants